[`flake8-bandit`] Implement `S4XX` suspicious import rules #8831

qdegraaf · 2023-11-24T11:22:36Z

Summary

Adds all S4XX rules to the flake8-bandit plugin port.

There is a lot of documentation to write, some tests can be expanded and implementation can probably be refactored to be more compact. As there is some discussion on whether this is actually useful. (See: #1646 (comment)), wanted to check which rules we want to have before I go through the process of polishing this up.

Test Plan

Fixtures for all rules based on flake8-bandit tests

Issue link

Refers: #1646

qdegraaf · 2024-01-02T18:14:17Z

@charliermarsh I added basic docs for all the rules. Ready for review or for culling anything which is redundant or unwanted in the end. Either way afterwards all S4XX rules in the issue can be ticked off and be close to finishing the plugin.

github-actions · 2024-01-02T18:23:31Z

`ruff-ecosystem` results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

ℹ️ ecosystem check detected linter changes. (+254 -0 violations, +0 -0 fixes in 8 projects; 33 projects unchanged)

DisnakeDev/disnake (+3 -0 violations, +0 -0 fixes)

ruff check --no-cache --exit-zero --preview

+ disnake/player.py:10:8: S404 `subprocess` module is possibly insecure
+ docs/conf.py:19:8: S404 `subprocess` module is possibly insecure
+ setup.py:17:16: S404 `subprocess` module is possibly insecure

apache/airflow (+130 -0 violations, +0 -0 fixes)

ruff check --no-cache --exit-zero --preview --select ALL

+ airflow/cli/commands/info_command.py:24:8: S404 `subprocess` module is possibly insecure
+ airflow/cli/commands/internal_api_command.py:23:8: S404 `subprocess` module is possibly insecure
+ airflow/cli/commands/standalone_command.py:22:8: S404 `subprocess` module is possibly insecure
+ airflow/cli/commands/webserver_command.py:23:8: S404 `subprocess` module is possibly insecure
+ airflow/configuration.py:30:8: S404 `subprocess` module is possibly insecure
+ airflow/executors/local_executor.py:30:8: S404 `subprocess` module is possibly insecure
... 89 additional changes omitted for rule S404
+ airflow/models/dag.py:28:8: S403 `pickle`, `cPickle`, `dill`, and `shelve` modules are possibly insecure
+ airflow/models/dagpickle.py:22:8: S403 `pickle`, `cPickle`, `dill`, and `shelve` modules are possibly insecure
+ airflow/models/taskinstance.py:36:8: S403 `pickle`, `cPickle`, `dill`, and `shelve` modules are possibly insecure
+ airflow/models/xcom.py:26:8: S403 `pickle`, `cPickle`, `dill`, and `shelve` modules are possibly insecure
+ airflow/operators/python.py:26:8: S403 `pickle`, `cPickle`, `dill`, and `shelve` modules are possibly insecure
+ airflow/operators/python.py:39:8: S403 `pickle`, `cPickle`, `dill`, and `shelve` modules are possibly insecure
... 23 additional changes omitted for rule S403
+ airflow/providers/amazon/aws/hooks/base_aws.py:338:14: S410 `lxml` is vulnerable to XML attacks
+ airflow/providers/ftp/hooks/ftp.py:21:8: S402 `ftplib` and related modules are considered insecure. Use SSH, SFTP, SCP, or another encrypted protocol.
+ airflow/providers/ftp/operators/ftp.py:23:6: S402 `ftplib` and related modules are considered insecure. Use SSH, SFTP, SCP, or another encrypted protocol.
+ airflow/providers/ftp/sensors/ftp.py:20:8: S402 `ftplib` and related modules are considered insecure. Use SSH, SFTP, SCP, or another encrypted protocol.
+ scripts/ci/testing/summarize_junit_failures.py:23:8: S405 `xml.etree` methods are vulnerable to XML attacks
+ scripts/in_container/check_junitxml_result.py:21:8: S405 `xml.etree` methods are vulnerable to XML attacks
+ tests/providers/ftp/sensors/test_ftp.py:20:6: S402 `ftplib` and related modules are considered insecure. Use SSH, SFTP, SCP, or another encrypted protocol.
+ tests/test_utils/get_all_tests.py:25:6: S405 `xml.etree` methods are vulnerable to XML attacks
... 110 additional changes omitted for project

bokeh/bokeh (+38 -0 violations, +0 -0 fixes)

ruff check --no-cache --exit-zero --preview --select ALL

+ examples/output/apis/server_document/flask_server.py:14:8: S404 `subprocess` module is possibly insecure
+ release/system.py:15:6: S404 `subprocess` module is possibly insecure
+ release/util.py:13:8: S403 `pickle`, `cPickle`, `dill`, and `shelve` modules are possibly insecure
+ scripts/hooks/install.py:1:8: S404 `subprocess` module is possibly insecure
+ scripts/hooks/protect_branches.py:3:6: S404 `subprocess` module is possibly insecure
+ scripts/hooks/uninstall.py:1:8: S404 `subprocess` module is possibly insecure
+ scripts/sri.py:7:6: S404 `subprocess` module is possibly insecure
... 30 additional changes omitted for rule S404
+ src/bokeh/sampledata/us_counties.py:45:8: S405 `xml.etree` methods are vulnerable to XML attacks
+ src/bokeh/sampledata/us_states.py:44:8: S405 `xml.etree` methods are vulnerable to XML attacks
... 29 additional changes omitted for project

freedomofpress/securedrop (+27 -0 violations, +0 -0 fixes)

ruff check --no-cache --exit-zero --preview

+ admin/bootstrap.py:23:8: S404 `subprocess` module is possibly insecure
+ admin/securedrop_admin/__init__.py:33:8: S404 `subprocess` module is possibly insecure
+ admin/tests/test_integration.py:4:8: S404 `subprocess` module is possibly insecure
+ admin/tests/test_securedrop-admin-setup.py:21:8: S404 `subprocess` module is possibly insecure
+ admin/tests/test_securedrop-admin.py:23:8: S404 `subprocess` module is possibly insecure
+ builder/tests/test_ossec_package.py:2:8: S404 `subprocess` module is possibly insecure
+ builder/tests/test_securedrop_deb_package.py:2:8: S404 `subprocess` module is possibly insecure
+ devops/scripts/verify-mo.py:22:8: S404 `subprocess` module is possibly insecure
+ install_files/ansible-base/roles/tails-config/files/securedrop_init.py:6:8: S404 `subprocess` module is possibly insecure
+ journalist_gui/journalist_gui/SecureDropUpdater.py:5:8: S404 `subprocess` module is possibly insecure
... 17 additional changes omitted for project

milvus-io/pymilvus (+1 -0 violations, +0 -0 fixes)

ruff check --no-cache --exit-zero --preview

+ pymilvus/client/__init__.py:3:8: S404 `subprocess` module is possibly insecure

rotki/rotki (+8 -0 violations, +0 -0 fixes)

ruff check --no-cache --exit-zero --preview

+ package.py:9:8: S404 `subprocess` module is possibly insecure
+ packaging/docker/entrypoint.py:7:8: S404 `subprocess` module is possibly insecure
+ rotkehlchen/tests/conftest.py:14:6: S404 `subprocess` module is possibly insecure
+ rotkehlchen/tests/integration/test_backend.py:1:8: S404 `subprocess` module is possibly insecure
+ tools/profiling/graph.py:36:8: S403 `pickle`, `cPickle`, `dill`, and `shelve` modules are possibly insecure
+ tools/profiling/sampler.py:3:8: S403 `pickle`, `cPickle`, `dill`, and `shelve` modules are possibly insecure
+ tools/profiling/trace.py:4:8: S403 `pickle`, `cPickle`, `dill`, and `shelve` modules are possibly insecure
+ tools/scripts/pylint_useless_suppression.py:17:8: S404 `subprocess` module is possibly insecure

scikit-build/scikit-build-core (+10 -0 violations, +0 -0 fixes)

ruff check --no-cache --exit-zero --preview

+ src/scikit_build_core/_shutil.py:7:8: S404 `subprocess` module is possibly insecure
+ src/scikit_build_core/builder/generator.py:4:8: S404 `subprocess` module is possibly insecure
+ src/scikit_build_core/cmake.py:8:8: S404 `subprocess` module is possibly insecure
+ src/scikit_build_core/errors.py:7:12: S404 `subprocess` module is possibly insecure
+ src/scikit_build_core/program_search.py:5:8: S404 `subprocess` module is possibly insecure
+ src/scikit_build_core/resources/_editable_redirect.py:6:8: S404 `subprocess` module is possibly insecure
+ tests/conftest.py:8:8: S404 `subprocess` module is possibly insecure
+ tests/test_dynamic_metadata.py:5:8: S404 `subprocess` module is possibly insecure
+ tests/test_name_main.py:2:8: S404 `subprocess` module is possibly insecure
+ tests/test_simple_pure.py:5:8: S404 `subprocess` module is possibly insecure

zulip/zulip (+37 -0 violations, +0 -0 fixes)

ruff check --no-cache --exit-zero --preview --select ALL

+ scripts/lib/check_rabbitmq_queue.py:4:8: S404 `subprocess` module is possibly insecure
+ scripts/lib/hash_reqs.py:5:8: S404 `subprocess` module is possibly insecure
+ scripts/lib/puppet_cache.py:5:8: S404 `subprocess` module is possibly insecure
+ scripts/lib/setup_venv.py:4:8: S404 `subprocess` module is possibly insecure
+ scripts/lib/sharding.py:6:8: S404 `subprocess` module is possibly insecure
+ scripts/lib/supervisor.py:5:6: S411 XMLRPC is vulnerable to remote XML attacks
+ scripts/lib/zulip_tools.py:14:8: S404 `subprocess` module is possibly insecure
... 24 additional changes omitted for rule S404
+ zerver/lib/markdown/__init__.py:30:6: S405 `xml.etree` methods are vulnerable to XML attacks
+ zerver/lib/markdown/include.py:4:6: S405 `xml.etree` methods are vulnerable to XML attacks
+ zerver/lib/markdown/nested_code_blocks.py:2:6: S405 `xml.etree` methods are vulnerable to XML attacks
... 27 additional changes omitted for project

Changes by rule (6 rules affected)

code	total	+ violation
S404	204	204
S403	33	33
S405	9	9
S402	4	4
S410	3	3
S411	1	1

charliermarsh

Thanks!

qdegraaf marked this pull request as ready for review November 24, 2023 16:15

qdegraaf force-pushed the feat/banditimports branch from 5e2bc90 to b5de9d9 Compare December 1, 2023 17:09

qdegraaf added 4 commits January 2, 2024 15:58

Add boilerplate for S4XX rules, add fixtures

8cf9d8f

Add first test, update schema, add first check

d606118

Add module level import rules

8f0fae5

Add remaining checks, refine tests based on bandit spec

5292b86

qdegraaf force-pushed the feat/banditimports branch from b5de9d9 to 5292b86 Compare January 2, 2024 14:58

qdegraaf added 2 commits January 2, 2024 19:05

Add docs

8edb486

Regen snapshots

78988da

charliermarsh added 2 commits January 3, 2024 13:09

Merge branch 'main' into feat/banditimports

f51dab4

Move to preview

c9aafeb

charliermarsh self-assigned this Jan 3, 2024

charliermarsh added rule Implementing or modifying a lint rule preview Related to preview mode features labels Jan 3, 2024

charliermarsh approved these changes Jan 3, 2024

View reviewed changes

charliermarsh enabled auto-merge (squash) January 3, 2024 18:19

charliermarsh merged commit 5c93a52 into astral-sh:main Jan 3, 2024
16 checks passed

qdegraaf deleted the feat/banditimports branch January 3, 2024 19:33

Porkepix mentioned this pull request Jan 12, 2024

ruff 0.1.12 Homebrew/homebrew-core#159717

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[`flake8-bandit`] Implement `S4XX` suspicious import rules #8831

[`flake8-bandit`] Implement `S4XX` suspicious import rules #8831

qdegraaf commented Nov 24, 2023 •

edited

Loading

qdegraaf commented Jan 2, 2024

github-actions bot commented Jan 2, 2024 •

edited

Loading

charliermarsh left a comment

[flake8-bandit] Implement S4XX suspicious import rules #8831

[flake8-bandit] Implement S4XX suspicious import rules #8831

Conversation

qdegraaf commented Nov 24, 2023 • edited Loading

Summary

Test Plan

Issue link

qdegraaf commented Jan 2, 2024

github-actions bot commented Jan 2, 2024 • edited Loading

ruff-ecosystem results

Linter (stable)

Linter (preview)

charliermarsh left a comment

Choose a reason for hiding this comment

[`flake8-bandit`] Implement `S4XX` suspicious import rules #8831

[`flake8-bandit`] Implement `S4XX` suspicious import rules #8831

qdegraaf commented Nov 24, 2023 •

edited

Loading

github-actions bot commented Jan 2, 2024 •

edited

Loading

`ruff-ecosystem` results